-
Notifications
You must be signed in to change notification settings - Fork 19
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weβll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: add hook for confirmed btc balance, inscription callouts #711
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good job @abdulhaseeb4239! π
Not a blocker, but I left a couple of suggestions below π
|
||
const fetchBtcAddressData = async () => btcClient.getAddressData(btcAddress); | ||
|
||
let confirmedBalance: number = 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let confirmedBalance: number = 0; | |
let confirmedBalance = 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: do we need to explicitly set the type here? I think we can rely on type inference here https://www.typescriptlang.org/docs/handbook/type-inference.html
src/locales/en.json
Outdated
@@ -982,7 +982,9 @@ | |||
"FAILED_TO_FINALIZE": "The inscription transaction failed to finalize. Please try again or contact support.", | |||
"SERVER_ERROR": "An unknown server error occurred. Please try again or contact support." | |||
}, | |||
"TOO_MANY_REPEATS": "You can only create up to 24 inscriptions in a single request" | |||
"TOO_MANY_REPEATS": "You can only create up to 24 inscriptions in a single request", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a valid point. Although this wasn't included in the current pull request, I've addressed it anyway π
@dhriaznov could you also make sure to maually test this PR and mark when tested, since @DuskaT021 won't be back until two more days |
@teebszet I'll pick it up today π |
@abdulhaseeb4239 could you resolve the conflicts pls? |
resolved |
A heads-up that the build just failed @abdulhaseeb4239 |
package-lock.json
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@abdulhaseeb4239 we should revert the package-lock.json
changes as they're not needed and they also cause the build failure in this PR
|
π PR Type
What kind of change does this PR introduce?
π Background
Xverse currently displays unconfirmed UTXOs as part of users' balance. This can lead to failed transaction attempts if users try to inscribe, thinking that they have sufficient funds, while some of their balance consists of unconfirmed UTXOs.
Issue Link:
ENG-3385
ENG-3416
π Changes
This PR depends on unreleased version from xverse-core PR
Impact:
πΌ Screenshot / πΉ Video
β Review checklist
Please ensure the following are true before merging: